Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix broken sdist #219

Merged
merged 2 commits into from
Oct 29, 2023
Merged

fix broken sdist #219

merged 2 commits into from
Oct 29, 2023

Conversation

szabolcsdombi
Copy link
Contributor

The sdist is broken because the setup.py cannot be called to get the package metadata because there is a copy operation at the root level.

When executing pip download PyGLM==2.7.0 --no-binary=:all: the downloaded source is not enough to get all the information about your package/module (for historical reasons, wheels do this better).

pip runs the setup.py to extract the metadata. This is not possible due to the copy operations in the root.
Insead of including those files in the MANIFEST.in as a hotfix. I removed the entire submodule and the copy operation from the setup.py as it would be correct.

Fixing it by other means, even adding the copy as a side effect for the build would just cause additional troubles.
Please consider merging this and maybe adding an acknowledgement to the author of the other repo.
stubs should clearly not be a submodule.

PS: glm should, that is 100% correct, submodules are great.

@Zuzu-Typ
Copy link
Owner

Zuzu-Typ commented Oct 29, 2023

Thank you for the help, @szabolcsdombi!
And sorry for dealing with this so late.

I realize that this was a bad approach. Obviously I shouldn't copy files around when using setup.py.
The reason why I did it this way is to be able to include the typing data with PyGLM, which, according to my research has to be in a specific directory "<module-name>-stubs" (in this case "glm-stubs") alongside the actual module. This seemed impossible to achive without using pyglm-typing as a submodule and moving the respective files to glm-stubs.

Unfortunately I don't know much about Python typing stubs, especially not for C-API modules. I guess I'll have to research before being able to correctly include the typing stubs. Either way, this PR is currently incomplete, since it removes the copy operations, but still expects typing stubs to exist in "glm-stubs". I'll have to remove typing stubs for the time being and address it at a later time.

-- EDIT
I guess I don't have to use a submodule, I already have a copy of "glm-stubs" in the project anyway, so indeed it does not make sense to have those copy operations. I'll merge the PR as is and try to fix some of the other issues so PyGLM is finally compatible with 3.12

os and shutil no longer required in setup.py
@Zuzu-Typ Zuzu-Typ merged commit 2b0b1da into Zuzu-Typ:master Oct 29, 2023
17 checks passed
@szabolcsdombi
Copy link
Contributor Author

You are experiencing issues with stubs due to they are a fairly new feature.
For example the stubs module seems not to work when you pip install -e . but do from pypi.
Adding a .pyi file in the root works with pip install -e . but not when installed from pypi.
There is a simple reason for this, it is complicated to explain though.

I tried all variations of stubs and I like the stubs module the most for objective reasons.
I even make them optional to include during an install.

https://github.com/szabolcsdombi/zengl/blob/97e55ae8a4ac02f808f8e927abccacfa51fdf5f3/setup.py#L10-L14

I ship no stubs for the web/pyodide version.

If you are into code formatters, black can actually format the pyi files too. you just have to run it on the stubs module.

There is also progress on including pyi files automatically: see: pypa/setuptools#3136 (comment)

I am going to test the master branch with this PR merged, and also with Pyodide.
I already used PyGLM on the web (here), but I was not able to propose a meta.yml for Pyodide due to this bug.
I am very grateful you have merged this, Thank You!

@Zuzu-Typ
Copy link
Owner

You are experiencing issues with stubs due to they are a fairly new feature.
For example the stubs module seems not to work when you pip install -e . but do from pypi.
Adding a .pyi file in the root works with pip install -e . but not when installed from pypi.
There is a simple reason for this, it is complicated to explain though.

Thanks for letting me know. I thought I was just missing the point or using a deprecated way of providing typing stubs (as I remember having difficulties getting information on how to use the "-stubs" module).

I already used PyGLM on the web (here), but I was not able to propose a meta.yml for Pyodide due to this bug.

That is most impressive! I didn't know using Python in the web using webassembly was already possible, let alone using OpenGL. Your work is really amazing (not just the PyWeek entry) and I'm honored that you use my humble library.

I am very grateful you have merged this, Thank You!

You're very welcome. Your help and input is always appreciated 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants